Repository navigation
(develop) Add a UI for fix file customization - #5294
TravisElless-NOAA merged 14 commits into
Conversation
… yaml
Add a top-level `fix:` section to the experiment yaml that overlays
user-supplied files on the installed fix tree. Entries are a flat
`dst: src` map with `dst` relative to the fix root; a plain source
replaces (file or directory), a glob source merges its matches into the
destination directory.
`setup_expt.py` builds the overlay in `${EXPDIR}/fix` with lazy
expansion: only directories on the path to an overlay entry become real
directories, everything else is a single symlink back to
`${HOMEglobal}/fix`. A manifest is written to `.fix_overlay.yaml`.
Missing sources, empty globs, escaping destinations and conflicting
entries are hard errors.
`config.base` now renders `FIXglobal` from the yaml, defaulting to
`${HOMEglobal}/fix`, so every `${FIXglobal}/...` reference follows the
overlay. The in-repo bypasses (`FIXgsi` and the three `config.aero`
chem paths) are switched from `${HOMEglobal}/fix` to `${FIXglobal}`.
Refs NOAA-EMC#5202
A glob merge followed by a single-file entry on the same leaf is the natural way to write "all of these, except this one from elsewhere", so duplicate leaves are now last-wins instead of an error. Each override is logged and recorded under `overrides:` in the manifest. Entries nested inside a wholesale-replaced directory remain an error.
…erge
Rename the experiment-yaml section from `fix:` to `fix_files:` and split
it into three sub-sections so the user states what they expect of the
fix tree, and setup_expt.py checks it:
replace: path must already exist (and be the same kind as the source)
add: path must not exist yet
merge: path must be an existing directory; every file in the source
directory, or every glob match, is linked in beside the
existing ones
A misspelled `replace` target or an `add` that would clobber a system
file is now an error instead of a silent no-op or overwrite. Globs are
only meaningful under `merge`, where they filter the source directory.
`replace`/`add` entries take precedence over `merge` matches regardless
of the order they are written in.
setup_expt.py ends with a summary block listing each entry with its
mode, destination, file count and `<-` source.
`fix_files:` now has only `replace:` and `add:`. A glob source is allowed under either when the destination is a directory: each match is linked into it under its own name and is checked individually against the rule of its sub-section (must already exist and match kind under `replace`, must not exist under `add`). A destination may take a list of sources so several globs can target one directory under the same sub-section. This removes the one place the interface let a misspelled source file land silently as an extra file, and with it the merge/replace precedence rules and the override records in the manifest: replace and add cannot claim the same path, so any duplicate is simply an error. The parent-is-a-file check moves before any directory is created, so a failed setup never leaves a partial overlay behind. The `add` error for a plain file onto an existing directory now suggests naming the full path of the new file.
A plain file inside a list was treated like a single plain source, so
`replace: {orog/C384: [<glob>, C384_mosaic.nc]}` failed as a kind
mismatch (directory dst, file src). A list now consistently means
"these go into that directory": every item, glob or plain, is linked
under its own name at <dst>/<basename> and checked individually.
A `fix_files: add:` section in the experiment yaml renders FIXglobal into config.base pointing at the overlay, links the requested files, and leaves untouched components as single links. Only `add:` is exercised because link_workflow.sh on the CI runner leaves dangling component links, so nothing "exists" for `replace:`.
Bash code analysis✅ shellcheck and shfmt found no issues. |
|
@AlexanderHrabski-NOAA I'll again voice my concern that this is a lot of code and extras to maintain when it seems like a two line command: seems sufficient. It's really nice and thoughtful work, but I personally just don't think we need this to be this fancy. |
|
@JessicaMeixner-NOAA I tried pretty hard to keep the diff in the existing workflow code small -- 15 lines of code excluding comments, the majority of which are either (a) setting a jinja default or (b) replacing instances of a hardcoded The RTOFS team's use case is a fair bit more than a 2-liner to set up, with a sizable and evolving set of changes to the default fix file tree. I'm counting over 50 additional files as of my last test, mostly to support the 8km ocean/ice resolution. We'd really benefit from something like this for the reasons I mentioned in #5202, especially when it comes to "configuration as code" for reproducibility among ourselves! It's very easy to forget how these fix file trees have evolved over time via manual linking as the diff from the default tree grows. By tracking the diff via the experiment's yaml and customizing the tree in the experiment's directory, these modifications are recorded and are isolated to a single experiment, all without having to maintain a full mirror of the fix file tree with various hand-edits ( |
emcbot
added
CI-Ursa-Ready
DavidHuber-NOAA
added
CI-Ursa-Passed
DavidHuber-NOAA
left a comment
DavidHuber-NOAA
left a comment
There was a problem hiding this comment.
I think the technical aspects of this PR are well developed, tested, and documented, so approve on that basis.
I will add that this adds some level of complexity, but for the user the change is minimal, so there seems to be some level of trade off. The design itself, however, is clean and may be expandable to other areas of the workflow. Some functions (e.g. _is_glob, _kind for sure (the latter being expandable to soft and hard links), perhaps others) may be served better in a higher level Python script or class (perhaps ecflow), but for now seem fine where they are.
emcbot
added
CI-Gaeac6-Ready
emcbot
added
the
CI-Gaeac6-Running
|
C96_gcafs_cycled FAILED on Gaeac6 (pipeline ID: 113072) In directory: Error Log Files: View Error Logs: (gcdas_fetch.log) (gcdas_fetch.log) This failure was detected automatically by global-workflow's CI/CD Pipeline |
|
C96_gcafs_cycled_noDA FAILED on Gaeac6 (pipeline ID: 113072) In directory: Error Log Files: View Error Logs: (gcdas_fetch.log) (gcdas_fetch.log) This failure was detected automatically by global-workflow's CI/CD Pipeline |
emcbot
added
CI-Gaeac6-Failed
|
Gaea failures are because HPSS is down and not caused by this PR |
|
C96_gcafs_cycled FAILED on Gaeac6 (pipeline ID: 113072) In directory: Error Log Files: View Error Logs: (gcdas_fetch.log) (gcdas_fetch.log) This failure was detected automatically by global-workflow's CI/CD Pipeline |
|
C96_gcafs_cycled_noDA FAILED on Gaeac6 (pipeline ID: 113072) In directory: Error Log Files: View Error Logs: (gcdas_fetch.log) (gcdas_fetch.log) This failure was detected automatically by global-workflow's CI/CD Pipeline |
|
C48_gsienkf_atmDA FAILED on Gaeac6 (pipeline ID: 113072) In directory: Error Log Files: View Error Logs: (gdas_prepatmanlbias.log) This failure was detected automatically by global-workflow's CI/CD Pipeline |
|
C48_ufsenkf_atmDA FAILED on Gaeac6 (pipeline ID: 113072) In directory: Error Log Files: View Error Logs: (gdas_prepatmanlbias.log) This failure was detected automatically by global-workflow's CI/CD Pipeline |
Review feedback: specify the default path in defaults.yaml so that overriding it
is something a user adds to their own experiment generation yaml, matching how
the rest of the configs express a path default.
So drop the default() filter from the four config.base.j2 and add FIXglobal to the
base section of each net's yaml/defaults.yaml. The ${HOMEglobal} reference passes
through the yaml untouched and is expanded by the shell when the config is sourced,
exactly as the ocn and ice template paths' ${PARMglobal} already is.
This also changes where setup_expt.py records the overlay path. A config is
rendered with host_plus_inputs_dict updated by yaml_dict[cfg_key], so the yaml
section wins; now that base carries a FIXglobal default, writing the overlay path
to host_plus_inputs_dict would have it silently discarded and the overlay built but
never used. Write it to yaml_dict['base'] instead, which is also the more honest
place for it: it is a base setting that this experiment overrides.
The test_setup.py end-to-end test covers this by asserting the shell-evaluated
config.base FIXglobal equals the overlay path.
Matches the treatment the template path entries got on the other branch: the default belongs in defaults.yaml, the explanation belongs in the user guide. Everything the comment said is already in the "Customizing fix files" section of docs/source/user_guide/setup.rst. Also place the key flush in the base key list rather than set off by a blank line, which was only there to separate it from the comment.
|
Updated the branch from develop and moved the defaults to |
# Description The pre-rendered input files that provide the MOM6 and CICE6 configuration are maintained as part of the UFS weather model submodule. If a set of experiments aim to modify the MOM6 and CICE6 configuration, a user tracks those changes by maintaining a fork of the UFS weather model. This PR provides makes the `mom_input`, `mom_data_table`, and `ice_in` template paths YAML-overrideable to allow users to track changes on these files (a) as part of their experiment's configuration and (b) without a fork of the weather model. The override is implemented via Jinja-templating these parameters in `config.ocn` and `config.ice` to make them overridable from the experiment setup YAML. In keeping with the established pattern for Jinja-templated variables with a default value, this PR also moves the default paths for these files from the runtime parsing utilities to their respective configuration files. Note that some additional MOM6 inputs, `mom_override` and `mom_layout`, are staged as fix files, so they require fix file path customization (#5294) to override their paths. See #5229 for comments on the feasibility of additional input file path overrides. There is no change to default behavior.
TravisElless-NOAA
left a comment
TravisElless-NOAA
left a comment
There was a problem hiding this comment.
Thanks for everyone's efforts with this PR! With minimal impact to the end user this gives us a chance to explore and expand future directions with the g-w. Therefore approving and merging this feature.
5 participants
Description
Forecasting system development can involve testing changes to fix files. This PR adds support for fix file customization as an experimental parameter, eliminating the need for manually re-linking global workflow's fix file installation
global-workflow/fix. Because these fix file edits are a parameter of the experiment, the UI takes the form of a new, optional section of the experiment setup yaml (at the level ofbase:) that reads like a diff from the global fix file installation, with a basic syntax ofdst: src:dstis relative to the rootfix/directory, andsrcis always absolute. Note that using jinja is an option, but it is not necessary.This UI is parsed by a new set of functions in
dev/workflow/fix_overlay.py, with error handling to enforce the UI's spec. Iffix_files:is included, the modified fix file tree is generated by a utility infix_overlay.pyintoEXPDIR/$pslot/fixso it exists alongside the rest of the experiment's configuration, and the workflow is pointed at it via{{ FIXglobal }}. The tree is linked lazily, so its generation time scales with the pieces of the tree the user touches, rather than the size of the entire global fix file tree.Unit tests are included for the new utilities. Because there are plenty of ways to break the UI, there a many error paths to test, leading to a relatively large number of new pytests. Despite their number, they are very fast to run.
There is no change in default behavior. This PR was co-authored by Claude Opus 5 and verified by myself.
A few loose ends:
A few paths in GDASApp use
{{ HOMEglobal }}/fixdirectly, bypassing the override, so a follow-up PR will be needed there for the submodule to support this feature @guillaumevernieres. Note that several uses of{{ HOMEglobal }}/fixin this repo were replaced as part of this PR.{{ FIXglobal }}is not available at the experiment-setup YAML's render time, so other, similar options likeSOCA_INPUT_FIX_DIRmust use${FIXglobal}in their paths to use the override. Note that the GDAS fix files can be customized via this interface, so it may be worth revisiting these options if the new interface completely covers their use cases.Resolves Support FIX file customization #5202
Type of change
Change characteristics
How has this been tested?
setup_expt.pyare correct.Remaining:
Checklist